Skip to content

Conversation

keval215
Copy link

Title: Fix type inconsistency in multiselect negotiate method and add temporary files to gitignore

Description:

Problem
This PR addresses a type inconsistency between the abstract interface and its implementation in the multiselect protocol negotiation:

• The IProtocolMuxer.negotiate() method in libp2p/abc.py has return type Optional[TProtocol] (allowing None)
• The implementation in libp2p/protocol_muxer/multiselect.py had return type TProtocol (not allowing None)
• There was a FIXME comment in the code acknowledging this mismatch

This inconsistency could cause type checking errors and potential runtime issues when the method legitimately returns None during failed negotiations.

Changes Made

  1. Fixed type annotation: Updated the negotiate method's return type in multiselect.py from TProtocol to Optional[TProtocol] to match the abstract interface
  2. Improved gitignore: Added .tmp.driveupload/ pattern to exclude temporary upload files from version control

Benefits
• ✅ Resolves type checking inconsistencies
• ✅ Aligns implementation with abstract interface contract
• ✅ Prevents accidental commits of temporary files
• ✅ Improves code maintainability and type safety

Testing
• Verified type annotations are consistent between interface and implementation
• Confirmed gitignore pattern correctly excludes temporary files
• No functional changes to existing behavior

Checklist
Code changes implemented
Type consistency verified
Gitignore updated

- Update multiselect.py negotiate() return type to match IMultiselectMuxer interface
- Allow TProtocol to be None in return type annotation
- Add .tmp.driveupload/ to .gitignore to exclude temporary upload files

Resolves type inconsistency between abstract interface and concrete implementation.
@@ -178,3 +178,6 @@ env.bak/
#lockfiles
uv.lock
poetry.lock

# Temporary upload files
.tmp.driveupload/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Funny I was using directory downloads/ for that
line

downloads/

@acul71
Copy link
Contributor

acul71 commented Aug 14, 2025

Hello @keval215 how are you doing? Are you still working on this?
Cause a new probabily closing PR is here #770

@acul71
Copy link
Contributor

acul71 commented Aug 14, 2025

Discussion here: #718

@seetadev
Copy link
Contributor

@acul71 : No response from @keval215 over 2+ weeks. We will wait this week and can safely close the PR.

@keval215
Copy link
Author

You can close the pr sorry for the delay

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants